New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Public Api for Register New Extensions for Rake Notes #14379
Conversation
|
||
# Registers new Annotations File Extensions | ||
# SourceAnnotationExtractor::Annotation.register_extensions("css", "scss", "sass", "less", "js") { |tag| /\/\/\s*(#{tag}):?\s*(.*)$/ } | ||
def self.regiter_extensions(*extensions, &block) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: regiter => register
Please add tests |
def self.directories | ||
@@directories ||= %w(app config db lib test) + (ENV['SOURCE_ANNOTATION_DIRECTORIES'] || '').split(',') | ||
end | ||
|
||
def self.extensions | ||
@@extensions ||= DEFAULT_EXTENSIONS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe you don't need defaults, you can just use the register method to create all defaults, wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds good 👍
we will need test cases. |
done, i guess 😄 |
|
||
register_extensions("builder", "rb", "rake") { |tag| /#\s*(#{tag}):?\s*(.*)$/ } | ||
register_extensions("css", "js") { |tag| /\/\/\s*(#{tag}):?\s*(.*)$/ } | ||
register_extensions("erb") { |tag| /<%\s*#\s*(#{tag}):?\s*(.*?)\s*%>/ } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are these all extensions supported by default? What about yml
for example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I removed all the extensions that can be added in the corresponding gems. I'm going to add the support for yml 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also I missed up .ruby
extension
… we have an API for register it in the corresponding gems
@robertomiranda : Thanks for your contribution! It looks like your tests are failing though. Would you mind having a look ? Could you also add a changelog entry please ? Also I don't know what do you guys think but I also thought about adding such feature for a while but at the railtie level. What do you think about providing a wrapper around config.annotations.register_extensions("coffee") do
/regex/
end |
app_file "app/assets/javascripts/application.js", "// TODO: note in js" | ||
app_file "app/assets/stylesheets/application.css", "// TODO: note in css" | ||
app_file "app/assets/stylesheets/application.css.scss", "// TODO: note in scss" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should keep scss
and sass
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevermind, this will be sass-rails job.
👍 for @robin850 idea. |
@robin850 👍 for the wrapper. I'm going to review the tests |
|
@@ -1,3 +1,7 @@ | |||
* Add Public Api for Register New Extensions for Rake Notes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we update it to:
Add public API to register new extensions for
rake notes
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be good to add an example on how to register new stuff, and maybe a guides update if we have something about rake notes in there.
All done |
Add Public Api for Register New Extensions for Rake Notes
Thanks!!! |
oops, next time please squash everything in a single commit, please 😁 |
+squash please. |
ref #14214